Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Remove secret usage from Vault PKI implementation for CA CSR #434

Merged
merged 4 commits into from
Jul 23, 2024

Conversation

DanielArndt
Copy link
Member

@DanielArndt DanielArndt commented Jul 19, 2024

Part of the solution for: #415

Currently, we store the CSR in a secret so that we can compare the certificate that is returned with the one we sent. However, we already store this CSR in the relation databag, and we are only ever requesting a single certificate.

Removing the dependence on secrets will simplify the solution for #415 because we won’t need to consider as many cases in the event that adding or updating the secret fails. Furthermore, it simplifies our code.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that validate the behaviour of the software
  • I validated that new and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@DanielArndt DanielArndt force-pushed the no-secrets-pki-ca-csr branch from 5f790b6 to 757df7b Compare July 19, 2024 16:47
@DanielArndt DanielArndt marked this pull request as ready for review July 22, 2024 11:33
@DanielArndt DanielArndt requested a review from a team as a code owner July 22, 2024 11:33
Copy link
Collaborator

@gruyaume gruyaume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you improve the PR description, only looking at the code changes does not make it obvious why we are doing them.

@DanielArndt
Copy link
Member Author

@gruyaume yeah, sorry, I added it to the Jira ticket but forgot to move it here.

@DanielArndt DanielArndt requested a review from gruyaume July 22, 2024 14:55
@DanielArndt DanielArndt enabled auto-merge (squash) July 22, 2024 23:57
@DanielArndt DanielArndt merged commit aa35974 into main Jul 23, 2024
12 checks passed
@DanielArndt DanielArndt deleted the no-secrets-pki-ca-csr branch July 23, 2024 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants